feat: allow port forward to deployment and service by mstruebing · Pull Request #2749 · kubernetes-client/javascript

mstruebing · 2026-01-17T13:32:58Z

cjihrig · 2026-01-17T21:42:59Z

examples/typescript/port-forward/port-forward-deployment.ts

+const namespace = process.argv[2] || 'default';
+const deploymentName = process.argv[3] || 'demo-deployment';
+const localPort = parseInt(process.argv[4] || '8080', 10);
+const remotePort = parseInt(process.argv[5] || '8080', 10);


Suggested change

const namespace = process.argv[2] || 'default';

const deploymentName = process.argv[3] || 'demo-deployment';

const localPort = parseInt(process.argv[4] || '8080', 10);

const remotePort = parseInt(process.argv[5] || '8080', 10);

const namespace = process.argv[2] ?? 'default';

const deploymentName = process.argv[3] ?? 'demo-deployment';

const localPort = parseInt(process.argv[4] ?? '8080', 10);

const remotePort = parseInt(process.argv[5] ?? '8080', 10);

cjihrig · 2026-01-17T21:44:33Z

examples/typescript/port-forward/port-forward-deployment.ts

+
+// This creates a local server that forwards traffic to a deployment in Kubernetes
+// by resolving the deployment to its first ready pod and port-forwarding to that pod.
+// Usage: npx ts-node port-forward-deployment.ts [namespace] [deploymentName] [localPort] [remotePort]


I don't think we should promote using ts-node. I believe it's unmaintained at this point. There is tsx, but also Node can run TypeScript directly now.

Today I learned! Thank you I didn't knew NodeJS is capable of running typescript direclty.
In case anyone else stumbles upon this comment: https://nodejs.org/en/learn/typescript/run-natively

cjihrig · 2026-01-17T21:46:07Z

examples/typescript/port-forward/port-forward-service.ts

+
+const forward = new k8s.PortForward(kc);
+
+const namespace = process.argv[2] || 'default';


Same comment on these lines about using ?? instead of ||.

I don't necessarily disagree, but why ?? instead of ||?

I think in general we should prefer ?? unless we explicitly want to default on all falsy values. In this case, process.argv[x] will be undefined if not set, so they should behave identically. I haven't benchmarked it, but I'd also expect ?? to be faster since it does slightly less work under the hood.

cjihrig · 2026-01-17T21:46:36Z

examples/typescript/port-forward/port-forward-service.ts

+
+// This creates a local server that forwards traffic to a service in Kubernetes
+// by resolving the service to its first ready pod and port-forwarding to that pod.
+// Usage: npx ts-node port-forward-service.ts [namespace] [serviceName] [localPort] [remotePort]


Same comment about ts-node here.

cjihrig · 2026-01-17T21:47:04Z

examples/port-forward-deployment.js

+const namespace = process.argv[2] || 'default';
+const deploymentName = process.argv[3] || 'demo-deployment';
+const localPort = parseInt(process.argv[4] || '8080', 10);
+const remotePort = parseInt(process.argv[5] || '8080', 10);


?? here as well please.

cjihrig · 2026-01-17T21:51:49Z

src/test/integration/portForward.ts

+                .catch((error) => {
+                    console.error('Deployment port forward error:', error.message);
+                    socket.destroy();
+                });


Can we use async-await and try...catch to be more consistent.

cjihrig · 2026-01-17T21:52:41Z

src/test/integration/portForward.ts

+        });
+
+        // Give the server a moment to start
+        await setTimeout(500);


Is this necessary? Once the listen() Promise above resolves, the server should be ready to serve traffic.

cjihrig · 2026-01-17T21:54:52Z

src/test/integration/portForward.ts

+        const serviceServer = net.createServer((socket) => {
+            portForward
+                .portForwardService(namespace, serviceName, [containerPort], socket, null, socket)
+                .catch((error) => {


Same comment here about using async-await and try...catch.

cjihrig · 2026-01-17T21:55:17Z

src/test/integration/portForward.ts

+        });
+
+        // Give the server a moment to start
+        await setTimeout(500);


Same question about this being necessary.

cjihrig · 2026-01-17T21:55:52Z

src/test/integration/portForward.ts

+        // Test connection to service via port-forward
+        for (let i = 0; i < 5; i++) {
+            try {
+                const response = await new Promise<string>((resolve, reject) => {


Is this logic all the same as above? If so, maybe it should be placed in its own function.

mstruebing · 2026-01-18T11:45:21Z

@cjihrig thanks for your review. I think I've addressed all your comments. I've made them in separate commits to easier keep track of what I already did and what I still need to do so it also be easy to review.

cjihrig

LGTM. Maybe we should give it another day for other reviewers to weigh in, but I'm fine with landing this.

cjihrig · 2026-01-18T20:14:31Z

examples/typescript/watch/watch-example.ts

@@ -1,40 +1,40 @@
-import * as k8s from '@kubernetes/client-node';
+// import * as k8s from '@kubernetes/client-node';
+import * as k8s from '../../../dist/index.js';


This looks like maybe it was left over from local testing.

I've removed it, good catch.
Prettier autoformatted the file so I would leave it in here.

davidgamero · 2026-01-19T21:54:05Z

src/test/integration/portForward.ts

+    assert.strictEqual(podsReady, true, 'Deployment pods did not become ready in time');
+
+    try {
+        // Test 1: Port forward to deployment


LGTM
can we move test 1, test2, test3 into named functions to organize main test func body?

deploymentSuccessTest(...) serviceSuccessTest(...) serviceNoSelectorTest(...)

davidgamero · 2026-01-19T21:56:12Z

src/test/integration/portForward.ts

+    // Wait for pods to be ready
+    console.log('Waiting for deployment pods to be ready...');
+    let podsReady = false;
+    for (let i = 0; i < 60; i++) {


nit the cycles/timouts are easier to understand when they are extracted into const

Suggested change

for (let i = 0; i < 60; i++) {

const MAX_POD_READY_CHECKS = 60;

const POD_READY_CHECK_POLL_DELAY_MS = 1000;

for (let i = 0; i < MAX_POD_READY_CHECKS; i++) {

davidgamero · 2026-01-19T21:56:31Z

src/test/integration/portForward.ts

+            console.log(`Deployment is ready with ${deployment.status?.readyReplicas} replicas`);
+            break;
+        }
+        await setTimeout(1000);


Suggested change

await setTimeout(1000);

await setTimeout(POD_READY_CHECK_POLL_DELAY_MS);

davidgamero · 2026-01-19T21:57:34Z

src/test/integration/portForward.ts

+}
+
+async function testPortForwardConnection(testPort: number, label: string): Promise<boolean> {
+    for (let i = 0; i < 5; i++) {


nit similar magic number or comment here

davidgamero

LGTM just a couple nits

mstruebing · 2026-01-21T09:24:48Z

Thanks @davidgamero I've adjusted the tests as you mentioned to be more modular :)

cjihrig

/lgtm
/approve

k8s-ci-robot · 2026-01-21T12:43:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cjihrig, mstruebing

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [cjihrig,mstruebing]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 17, 2026

k8s-ci-robot requested review from brendandburns and cjihrig January 17, 2026 13:33

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 17, 2026

feat: allow port forward to deployment and service

3dffec5

mstruebing force-pushed the 435-port-foward branch from 8d0ba93 to 3dffec5 Compare January 17, 2026 13:56

cjihrig reviewed Jan 17, 2026

View reviewed changes

mstruebing added 5 commits January 18, 2026 11:25

chore: adjust exampels to use ?? instead of || and use node directly

3f250b8

chore: remove unnecessary timeout

d8e9ed0

chore: extract duplicated logic into reusable function

bdda1c1

chore: use async-await and try-catch instead of promise

0742245

chore: make typescript happy

0ad0554

cjihrig approved these changes Jan 18, 2026

View reviewed changes

chore: revert changes to watch example

d89f2f2

davidgamero reviewed Jan 19, 2026

View reviewed changes

mstruebing added 2 commits January 21, 2026 08:09

chore: replace magic numbers

ae9b57c

test: restructure test to be more readable

87b79a4

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 21, 2026

cjihrig approved these changes Jan 21, 2026

View reviewed changes

k8s-ci-robot assigned cjihrig Jan 21, 2026

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 21, 2026

k8s-ci-robot merged commit 25dc02b into main Jan 21, 2026
10 checks passed


		const forward = new k8s.PortForward(kc);

		const namespace = process.argv[2] \|\| 'default';

-    for (let i = 0; i < 60; i++) {
+    const MAX_POD_READY_CHECKS = 60;
+    const POD_READY_CHECK_POLL_DELAY_MS = 1000;
+    for (let i = 0; i < MAX_POD_READY_CHECKS; i++) {

	await setTimeout(1000);
	await setTimeout(POD_READY_CHECK_POLL_DELAY_MS);

Conversation

mstruebing commented Jan 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mstruebing commented Jan 18, 2026

Uh oh!

cjihrig left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidgamero left a comment

Choose a reason for hiding this comment

Uh oh!

mstruebing commented Jan 21, 2026

Uh oh!

cjihrig left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jan 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mstruebing commented Jan 17, 2026 •

edited

Loading